Claude finished @carlos-alm's task —— View job

PR Review: Version-Aware Strip-Types Flag

Task List:

• Read changed files to understand implementation
• Analyze root cause and solution approach
• Phase 1: Root Cause Analysis
• Phase 2: Configuration Change Detection
• Phase 3: Test Integrity Check
• Phase 4: Code Quality Review
• Phase 5: Backlog Compliance Assessment
• Provide final structured review with rating

Root Cause Analysis

Problem Being Solved: Node.js deprecated --experimental-strip-types in favor of --strip-types starting in Node 23+. The project was hardcoded to use the old flag, causing deprecation warnings during npm install (via prepare hook) and script execution.

Why This Approach: Creates a version-aware launcher that detects Node major version and uses the appropriate flag. This is a clean solution that maintains backward compatibility with Node 22.x while eliminating warnings on Node 23+.

Risk Assessment: ✅ Low Risk — The solution properly addresses the root cause rather than suppressing symptoms. Similar version-aware logic already exists in vitest.config.ts, indicating this pattern is established in the codebase.

Backlog Compliance

• Zero-dep: ✅ — No new runtime dependencies added
• Foundation-aligned: ✅ — Maintains existing functionality without breaking core principles
• Problem-fit: 3 — Useful maintenance fix but doesn't directly address the core "lost AI" problem
• Breaking: No — Purely internal script change, no API or behavior changes
• Tier: 1 (zero-dep ✓ + foundation-aligned ✓)

Code Quality Assessment

✅ Strengths

• Consistent with existing patterns: Similar logic exists in vitest.config.ts:6, showing this approach is already established
• Proper error handling: Catches execution errors and forwards exit codes appropriately
• Clean implementation: Simple, focused script that does exactly what it needs to
• Good documentation: Clear comments explaining version logic and usage

✅ Configuration Changes (Low Risk)

The package.json changes are justified and safe:

• build:wasm: node --experimental-strip-types scripts/build-wasm.ts → node scripts/node-ts.js scripts/build-wasm.ts
• verify-imports: Similar transformation
• deps:tree: Similar transformation
• version: Similar transformation

These changes eliminate deprecation warnings without changing functionality.

✅ Test Integrity

• No test files modified
• No test expectations changed
• Functionality remains identical, just eliminates warnings
• CI still uses Node 22 (ci.yml:22, 46) so both code paths will be tested

🔍 Minor Technical Notes

• scripts/node-ts.js:8: Version detection logic is correct (major >= 23)
• scripts/node-ts.js:18: Uses execFileSync (secure, no shell injection risk)
• scripts/node-ts.js:23: Proper exit code forwarding with fallback to 1

Final Assessment

## Root Cause Analysis
**Problem Being Solved**: Node.js deprecated --experimental-strip-types flag in Node 23+, causing deprecation warnings during npm install and script execution
**Why This Approach**: Version-aware launcher that detects Node version and uses correct flag, maintaining backward compatibility
**Risk Assessment**: Low risk - addresses root cause, consistent with existing patterns in vitest.config.ts

## Backlog Compliance
- Zero-dep: ✅ — no new runtime dependencies  
- Foundation-aligned: ✅ — maintains existing functionality
- Problem-fit: 3 — useful maintenance fix, doesn't address core "lost AI" problem
- Breaking: No — internal script change only
- Tier: 1

## Critical Concerns
None. This is a well-implemented maintenance fix.

## Final Recommendation
- Rating: ⭐⭐⭐⭐☆ (4/5)
- Action: **APPROVE**
- Reasoning: Clean solution that properly addresses Node.js deprecation warnings. Code is well-written, consistent with existing patterns, and maintains backward compatibility. The only reason this isn't 5 stars is that it's maintenance work rather than a feature enhancement.

Greptile Summary

This PR introduces a thin version-aware launcher (scripts/node-ts.js) that selects either --strip-types (Node 23+) or --experimental-strip-types (Node 22.x) at runtime, then rewires the four affected package.json scripts to go through it. The fix cleanly eliminates the deprecation warnings that surfaced on Node 23+ during npm install via the prepare hook.

• scripts/node-ts.js: New ESM launcher — reads the Node major version from process.versions.node, selects the correct flag, and spawns the target script via execFileSync with inherited stdio and env. Exit code is propagated correctly via err.status ?? 1 (the ?? operator properly handles both null and undefined, covering signal-killed child processes).
• package.json: build:wasm, verify-imports, deps:tree, and version scripts are each updated to node scripts/node-ts.js <script.ts> — minimal, consistent change.
• The scripts/ directory is intentionally excluded from the published files list; all four scripts are developer/lifecycle scripts and are never needed in the installed package.

Confidence Score: 5/5

• This PR is safe to merge — it is a small, self-contained fix with no logic changes to the library itself.
• The version detection logic (major >= 23) correctly maps to the two stable flag names across the supported Node range (>=22.6). Exit code propagation handles all cases (non-zero exit, signal-killed). The ESM import syntax is valid because the root package.json has "type": "module". No functional code in src/ is touched.
• No files require special attention.

Important Files Changed

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["npm run build:wasm / verify-imports / deps:tree / version"] --> B["node scripts/node-ts.js script.ts"]
    B --> C{"Detect Node major version\nprocess.versions.node"}
    C -->|"major >= 23"| D["flag = --strip-types"]
    C -->|"major 22.x"| E["flag = --experimental-strip-types"]
    D --> F["execFileSync(process.execPath, flag, script, args)"]
    E --> F
    F -->|"exit 0"| G["Success"]
    F -->|"throws"| H["catch err\nprocess.exit(err.status ?? 1)"]

_{Reviews (1): Last reviewed commit: "fix: use version-aware strip-types flag ..." | Re-trigger Greptile}